Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix VitessAware system variables of type boolean return NULL when MySQL is not involved #7353

Merged
merged 14 commits into from
Jan 28, 2021

Conversation

frouioui
Copy link
Member

@frouioui frouioui commented Jan 23, 2021

Description

This PR fixes #7301. As detailed in the issue, selecting a VitessAware system variable without involving MySQL/Vttablets might return a nil result.

Related Issue(s)

Checklist

  • Should this PR be backported?
  • Tests were added or are not required
  • Documentation was added or is not required

Impacted Areas in Vitess

Components that this PR will affect:

  • Query Serving
  • VReplication
  • Cluster Management
  • Build
  • VTAdmin

Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
@frouioui frouioui changed the title fixed want/result order in executor_select_test assertions Fix VitessAware system variables of type boolean return NULL when MySQL is not involved Jan 23, 2021
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
@frouioui
Copy link
Member Author

frouioui commented Jan 23, 2021

The new test case:

func TestSelectSingleVitessAwareVariable(t *testing.T) {
Will voluntarily fail until the issue is not resolved.

Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
@frouioui
Copy link
Member Author

To reproduce the issue we can use the 101_initial_cluster.sh example. Once our sample cluster is running, if we select for instance the @@autocommit variable, the result will be as followed:

mysql> select @@autocommit;
+--------------+
| @@autocommit |
+--------------+
|         NULL |
+--------------+
1 row in set (0.01 sec)

A fix has been introduced in 34ffa84 allowing the EvalEngine to handle 32-bit variables. It seems that 64-bit variables are used by queries that needs to communicate with MySQL, and 32-bit are used for VTGate-only query.

Unit tests have been modified accordingly.

The behavior now is as followed:

examples/local $  ./101_initial_cluster.sh

...

examples/local $  mysql

...

mysql> select @@autocommit;
+--------------+
| @@autocommit |
+--------------+
|            1 |
+--------------+
1 row in set (0.00 sec)

Ran on:

Version: da7611d87 (Git branch 'fix-7301') built on Sat Jan 23 18:05:31 CET 2021 by pflorent@florents-macbook-pro.home using go1.15.7 darwin/amd64

@frouioui frouioui marked this pull request as ready for review January 23, 2021 17:25
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
@frouioui frouioui requested a review from sougou as a code owner January 27, 2021 14:04
frouioui and others added 5 commits January 27, 2021 20:38
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
@frouioui
Copy link
Member Author

Up until now, boolean bind variables were typed int32 within vtgate. Though, MySQL uses 64-bit, meaning that we were facing a type inconsistency problem.

For instance, the following query returns a type int32:

select @@autocommit

Whereas the following query returns a type int64:

select @@autocommit from my_table

To suppress this problem, @systay and I decided upon using int64 variables for boolean bind variables. Changes were performed in both 0294465 and 2d91120.

@frouioui frouioui requested a review from systay January 27, 2021 21:27
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
@systay systay merged commit a05c153 into vitessio:master Jan 28, 2021
@askdba askdba added this to the v10.0 milestone Feb 8, 2021
@frouioui frouioui deleted the fix-7301 branch July 12, 2023 08:36
@frouioui frouioui restored the fix-7301 branch July 12, 2023 08:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

VitessAware system variables of type boolean return NULL when the query is not passed to MySQL
3 participants